fix(tests): anchor calendar tests to past dates to avoid user-status side effects#426
Conversation
📝 WalkthroughWalkthroughTests in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/actual_tests/calendar_test.py (1)
89-97: 🧹 Nitpick | 🔵 TrivialRename
now— it's 400 days in the past.Minor readability nit: the variable name
nowactively misleads readers of the lifecycle block, since everydtstart=nowis in fact ~13 months ago. A name likepast(matchingtest_create_delete) oranchorkeeps the fix intent visible.♻️ Proposed rename
- # Use a past anchor so these events don't trigger Nextcloud's - # calendar-status automation (see test_create_delete for details). - now = datetime.datetime.now() - datetime.timedelta(days=400) - calendar.save_event( - dtstart=now, - dtend=now + datetime.timedelta(hours=2), + # Use a past anchor so these events don't trigger Nextcloud's + # calendar-status automation (see test_create_delete for details). + past = datetime.datetime.now() - datetime.timedelta(days=400) + calendar.save_event( + dtstart=past, + dtend=past + datetime.timedelta(hours=2), summary="Original Title", ) @@ calendar.save_event( - dtstart=now, - dtend=now + datetime.timedelta(hours=1), + dtstart=past, + dtend=past + datetime.timedelta(hours=1), summary="Event with spëcial chars: <>&\"'", )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/actual_tests/calendar_test.py` around lines 89 - 97, The variable now is misleading because it represents a timestamp 400 days in the past; rename it to a clearer identifier (e.g., past or anchor) wherever it is declared and used in this test block so the intent is obvious—update the assignment and all usages passed to calendar.save_event (dtstart, dtend) and any other references in the test lifecycle; ensure the new name matches the existing rename in test_create_delete for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@tests/actual_tests/calendar_test.py`:
- Around line 89-97: The variable now is misleading because it represents a
timestamp 400 days in the past; rename it to a clearer identifier (e.g., past or
anchor) wherever it is declared and used in this test block so the intent is
obvious—update the assignment and all usages passed to calendar.save_event
(dtstart, dtend) and any other references in the test lifecycle; ensure the new
name matches the existing rename in test_create_delete for consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4542217f-21b3-4e72-992c-70d359b87102
📒 Files selected for processing (1)
tests/actual_tests/calendar_test.py
oleksandr-nc
force-pushed
the
fix/calendar-test-past-dates
branch
from
0da343d to
adc82a2
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/actual_tests/calendar_test.py (1)
135-177: 🧹 Nitpick | 🔵 TrivialLGTM — search windows shifted coherently with
base.All
start/endbounds are expressed relative tobase, so anchoring in the past doesn't affect the REPORT logic under test. RenamedDay 0/+1/+7summaries are unambiguous and the inclusion/exclusion assertions line up with each window.Optional nit: per the
tests/**guideline on adequate assertion messages, consider adding messages to thein/not inchecks (e.g.assert "Day +7 Event" not in summaries, f"unexpected: {summaries}") to make CI failures self-diagnosing — but this matches existing style in the file, so feel free to skip.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/actual_tests/calendar_test.py` around lines 135 - 177, Tests currently assert membership with bare assert "X" in summaries / not in summaries which can yield unclear failures; update the three membership assertions around the calendar.search calls (referencing the variable base and the calendar.search/results handling) to include informative assertion messages (e.g., append , f"unexpected summaries: {summaries}") so CI failures show the actual summaries contents while keeping the same assertions and semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@tests/actual_tests/calendar_test.py`:
- Around line 135-177: Tests currently assert membership with bare assert "X" in
summaries / not in summaries which can yield unclear failures; update the three
membership assertions around the calendar.search calls (referencing the variable
base and the calendar.search/results handling) to include informative assertion
messages (e.g., append , f"unexpected summaries: {summaries}") so CI failures
show the actual summaries contents while keeping the same assertions and
semantics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2f36b199-f0b0-4167-b375-9e08f8c0cd25
📒 Files selected for processing (1)
tests/actual_tests/calendar_test.py
bigcat88
force-pushed
the
fix/calendar-test-past-dates
branch
from
adc82a2 to
b6cbd15
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/actual_tests/calendar_test.py (1)
158-158: 🧹 Nitpick | 🔵 TrivialUpdate the stale “today” comment.
Line 158 still refers to “today”, but the test is now intentionally anchored to
Day 0.Proposed comment fix
- # Search for today only + # Search for day 0 only🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/actual_tests/calendar_test.py` at line 158, Replace the stale inline comment "# Search for today only" with a comment that correctly reflects the test's anchor to Day 0 (for example "# Search for Day 0 only") so the comment matches the test's intent; update the comment near the calendar test where the original string "# Search for today only" appears.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@tests/actual_tests/calendar_test.py`:
- Line 158: Replace the stale inline comment "# Search for today only" with a
comment that correctly reflects the test's anchor to Day 0 (for example "#
Search for Day 0 only") so the comment matches the test's intent; update the
comment near the calendar test where the original string "# Search for today
only" appears.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 22a371ef-5123-4dcf-9b88-0a63ed01ba35
📒 Files selected for processing (1)
tests/actual_tests/calendar_test.py
1 participant
Calendar tests create events anchored to
now. Nextcloud's user-status app auto-sets a📅 In a meetingicon for admin when any event overlaps the next 5 minutes, and caches that for 5 min.user_status_test.py::test_get_status_async[app_async-1 2 3]runs shortly after and breaks onassert status_icon is None.Moved the three offending events ~400 days back. CRUD semantics unchanged, side-effect gone.
Summary by CodeRabbit